fix table name and add unit test for colors passed via .uns #413

ArneDefauw · 2025-01-28T10:26:12Z

spatialdata_plot.pl.render._render_labels still had sdata.table , changed to sdata.tables[table_name], otherwise colors passed via .uns where ignored.

also in

spatialdata-plot/src/spatialdata_plot/pl/utils.py

Line 854 in 43f25f6

    
           if adata is not None and cluster_key in adata.uns and f"{cluster_key}_colors" in adata.uns:

I would remove the check cluster_key in adata.uns. Because of this I had to set

    `sdata_blobs[ "other_table" ].uns[ "category" ] = "__value__"`

as a placeholder in https://github.com/ArneDefauw/spatialdata-plot/blob/b5cb0260c32a859783f87f10b8af05dc26feab76/tests/pl/test_render_labels.py#L219 .

I also stumbled upon a subtle bug while testing this feature. With the small fix in this PR, one can pass colors via .uns[ f"{cluster_key}_colors" ], and all works as expected, see
https://github.com/ArneDefauw/spatialdata-plot/blob/b5cb0260c32a859783f87f10b8af05dc26feab76/tests/pl/test_render_labels.py#L219

We get:

But if we do a bounding box query, we get:

While we expect:

In short, this is caused by https://github.com/scverse/spatialdata/blob/03d3be80fad69ff54097e90a9e80ad02e9e0e242/src/spatialdata/_utils.py#L203

also see scverse/anndata#997

For details see https://github.com/ArneDefauw/spatialdata-plot/blob/b5cb0260c32a859783f87f10b8af05dc26feab76/tests/pl/test_render_labels.py#L227 and possible workaround https://github.com/ArneDefauw/spatialdata-plot/blob/b5cb0260c32a859783f87f10b8af05dc26feab76/tests/pl/test_render_labels.py#L252

Note that being able to plot a subset in the same colors as the orignal is a rather common use case. However, I am not sure where to fix. Maybe documenting the workaround suffices for now.

for more information, see https://pre-commit.ci

…ot into fix_table_name

for more information, see https://pre-commit.ci

…ot into fix_table_name

LucaMarconato · 2025-01-28T13:41:44Z

Hi @ArneDefauw, thanks for the contribution. Two initial comments:

could you please open one issue describing what the PR addresses? This makes it easier to review and keeps a cleaner history of what a PR introduces. At the moment it's not immediate and one has to go and check the code and discussion. Or in alternative, without having to open an issue, can you please add a summary at the beginning of the PR of the issues being addressed? Thanks a lot!
I see some tests are not passing, one seems to be due to a difference between an old plot and the new one, while other seems to be due to raised exception. Do you have fix for them or comments on them?

Thanks!

ArneDefauw · 2025-01-28T14:48:17Z

Hi @LucaMarconato ,

I've linked an issue #414.

For the failed unit tests, there was a KeyError, because I overlooked that table_name can be None. I changed it in the PR.

For me all unit tests fail locally on the main branch of spatialdata-plot, so it is difficult to test. Therfore, I did not include base images for the unit tests I've added, because they would probably fail anyway in the CI/CD testing environment.

The unit tests I've added are for documentation of the issue, some of them can probably be removed when merging to main.

LucaMarconato · 2025-01-28T23:07:11Z

Thanks for updating the PR! Regarding the failing tests, could you please give a try to the approach described here? https://github.com/scverse/spatialdata-plot/blob/main/docs/contributing.md

Locally generally the tests are reproducible on Ubuntu, but for other OS I use a Docker image, as described in the link above.

LucaMarconato · 2025-01-28T23:08:08Z

My configuration with PyCharm is described here #397.

for more information, see https://pre-commit.ci

codecov-commenter · 2025-01-29T10:31:41Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.23%. Comparing base (43f25f6) to head (be331f3).
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #413      +/-   ##
==========================================
+ Coverage   83.83%   84.23%   +0.40%     
==========================================
  Files           8        8              
  Lines        1732     1732              
==========================================
+ Hits         1452     1459       +7     
+ Misses        280      273       -7

Files with missing lines	Coverage Δ
src/spatialdata_plot/pl/utils.py	`78.69% <ø> (+0.69%)`	⬆️

ArneDefauw · 2025-01-29T10:40:59Z

I managed to fix the tests (using a centOS server).

Note that I had to move this line

spatialdata-plot/tests/pl/test_render_labels.py

Line 16 in 478ab27

RNG = np.random.default_rng(seed=42)

inside

spatialdata-plot/tests/pl/test_render_labels.py

Line 217 in 478ab27

    
           def _make_tablemodel_with_categorical_labels(self, sdata_blobs, labels_name: str):

, respectively

spatialdata-plot/tests/pl/test_render_labels.py

Line 237 in 478ab27

    
           def test_plot_can_annotate_labels_with_table_layer(self, sdata_blobs: SpatialData):

to have the same results when running one test versus all the tests, because its internal state was moving forward due to multiple calls to it. Because of this I also had to update this figure https://github.com/scverse/spatialdata-plot/blob/main/tests/_images/Labels_can_annotate_labels_with_table_layer.png

LucaMarconato · 2025-01-30T15:54:20Z

Fantastic! Thanks a lot!

ArneDefauw and others added 9 commits January 28, 2025 10:58

fix table name and add unit test

b5cb026

[pre-commit.ci] auto fixes from pre-commit.com hooks

0f2663e

for more information, see https://pre-commit.ci

uncomment matplotlib use agg

0d4867c

Merge branch 'fix_table_name' of github.com:ArneDefauw/spatialdata-pl…

6c04b11

…ot into fix_table_name

category not deterministic in unit test

2dfb47c

[pre-commit.ci] auto fixes from pre-commit.com hooks

bd39e7e

for more information, see https://pre-commit.ci

matplotlib

39038e9

Merge branch 'fix_table_name' of github.com:ArneDefauw/spatialdata-pl…

2f449f4

…ot into fix_table_name

import

5af65aa

ArneDefauw mentioned this pull request Jan 28, 2025

Plot categorical color when colors are provided via .uns #414

Open

fix for keyerror

1c58719

ArneDefauw and others added 3 commits January 29, 2025 09:08

tests

6a33304

move rng, fix tests

6d9a590

[pre-commit.ci] auto fixes from pre-commit.com hooks

be331f3

for more information, see https://pre-commit.ci

timtreis added priority: medium labels 🏷️ Anything related to Labels labels Jan 29, 2025

LucaMarconato mentioned this pull request Jan 30, 2025

render_labels color plot no longer working #403

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix table name and add unit test for colors passed via .uns #413

fix table name and add unit test for colors passed via .uns #413

Uh oh!

ArneDefauw commented Jan 28, 2025

Uh oh!

LucaMarconato commented Jan 28, 2025

Uh oh!

ArneDefauw commented Jan 28, 2025 •

edited

Loading

Uh oh!

LucaMarconato commented Jan 28, 2025

Uh oh!

LucaMarconato commented Jan 28, 2025

Uh oh!

codecov-commenter commented Jan 29, 2025 •

edited

Loading

Uh oh!

ArneDefauw commented Jan 29, 2025

Uh oh!

LucaMarconato commented Jan 30, 2025

Uh oh!

Uh oh!

fix table name and add unit test for colors passed via .uns #413

Are you sure you want to change the base?

fix table name and add unit test for colors passed via .uns #413

Uh oh!

Conversation

ArneDefauw commented Jan 28, 2025

Uh oh!

LucaMarconato commented Jan 28, 2025

Uh oh!

ArneDefauw commented Jan 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LucaMarconato commented Jan 28, 2025

Uh oh!

LucaMarconato commented Jan 28, 2025

Uh oh!

codecov-commenter commented Jan 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ArneDefauw commented Jan 29, 2025

Uh oh!

LucaMarconato commented Jan 30, 2025

Uh oh!

Uh oh!

ArneDefauw commented Jan 28, 2025 •

edited

Loading

codecov-commenter commented Jan 29, 2025 •

edited

Loading